Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and extend tests #48

Merged
merged 50 commits into from
Aug 28, 2023
Merged

Fix and extend tests #48

merged 50 commits into from
Aug 28, 2023

Conversation

FelixMau
Copy link
Contributor

@FelixMau FelixMau commented Aug 16, 2023

Closes #43

Please note that two test cannot be completed yet since they depend on changes from other branches and modules.

  • test_build_tabular_datapackage_from_adapter requires changes in data_adapter to receive only one column per name and moreover needs refactoring from Merge and write periods #45 for MultiIndex Timerseries data (@henhuy if this is still required after updating data_adapter?)
  • test_read_datapackage Relies on Implementing Multiperiod functionality for Scalar values first.

FelixMau and others added 13 commits August 10, 2023 13:20
- Import statements outdated
- Adapter choices weren't appropriate
- `Mapper` call changed
- `test_get_sequence_name` needs to be checked for consistency with requirements but has been updated to succeed.
- Import statements outdated
- Adapter choices weren't appropriate
- `Mapper` call changed
- `test_get_sequence_name` needs to be checked for consistency with requirements but has been updated to succeed.
@FelixMau
Copy link
Contributor Author

It seems that tests are failing in data_adapter for unsupported operand
On my machine the tests run without errors. I'd be glad for feedback on what to do here.

@nailend
Copy link
Contributor

nailend commented Aug 16, 2023

I also don't understand what is happening. The tests fail for python 3.8 and 3.10 for different reasons. I tried to fix the environment variable thing but dont know if I just disimproved it... @henhuy can u have a look?

FelixMau and others added 12 commits August 17, 2023 09:24
- Import statements outdated
- Adapter choices weren't appropriate
- `Mapper` call changed
- `test_get_sequence_name` needs to be checked for consistency with requirements but has been updated to succeed.
- Import statements outdated
- Adapter choices weren't appropriate
- `Mapper` call changed
- `test_get_sequence_name` needs to be checked for consistency with requirements but has been updated to succeed.
@nailend
Copy link
Contributor

nailend commented Aug 17, 2023

@FelixMau can you please check on the missing csvfiles in test_build_datapackage?

The list of ~300 mapping-warnings is impressive. Maybe we should specify these, with information about the process name to make them more usefull?

@FelixMau
Copy link
Contributor Author

FelixMau commented Aug 22, 2023

@FelixMau can you please check on the csvfiles in test_build_datapackage?

Do you mean because of missing period merging? I think we have to decide on how the inputs should look like first (in #45)

The list of ~300 mapping-warnings is impressive.

Yes indeed impressive and less usefull, will have a look into that!

@nailend
Copy link
Contributor

nailend commented Aug 24, 2023

Do you mean because of missing period merging? I think we have to decide on how the inputs should look like first (in #45)

https://github.com/sedos-project/data_adapter_oemof/actions/runs/5888510361/job/15969855183?pr=48#step:9:538

Because of the missing csv

@FelixMau
Copy link
Contributor Author

FelixMau commented Aug 28, 2023

  • Add Todo for reducing warnings
    Warnings reduced significantly already and will be reduced further as better tests are written.

Copy link
Contributor

@nailend nailend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all fine! Thanks a lot for fixing this!

@@ -152,7 +155,8 @@ def get_foreign_keys(struct: list, mapper: Mapper, components: list) -> list:
}
)
else:
# Most likely the field may be a Timeseries in this case, but it is a scalar or unused.
# Most likely the field may be a Timeseries in this case, but it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is a bit confusing, improve or remove?

@FelixMau FelixMau merged commit 3f741bd into dev Aug 28, 2023
3 checks passed
@FelixMau FelixMau deleted the fix-and-extend-tests branch August 28, 2023 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix test
3 participants